Skip to content

BUG-24408 Series.dt does not maintain own copy of index #24426

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Dec 26, 2018
Merged

BUG-24408 Series.dt does not maintain own copy of index #24426

merged 6 commits into from
Dec 26, 2018

Conversation

JustinZhengBC
Copy link
Contributor

Previously, the dt accessor maintains its own copy of the index, so in-place operations would cause the indexes to be out of sync. This PR changes the accessor so it just gets the index from its parent.

@codecov
Copy link

codecov bot commented Dec 25, 2018

Codecov Report

Merging #24426 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24426      +/-   ##
==========================================
+ Coverage    92.3%    92.3%   +<.01%     
==========================================
  Files         163      163              
  Lines       51954    51953       -1     
==========================================
  Hits        47957    47957              
+ Misses       3997     3996       -1
Flag Coverage Δ
#multiple 90.71% <100%> (-0.01%) ⬇️
#single 42.99% <33.33%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/indexes/accessors.py 91% <100%> (-0.09%) ⬇️
pandas/util/testing.py 87.84% <0%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a09706...d21a893. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 25, 2018 •
edited
Loading

Codecov Report

Merging #24426 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24426      +/-   ##
==========================================
- Coverage    92.3%    92.3%   -0.01%     
==========================================
  Files         163      163              
  Lines       51966    51965       -1     
==========================================
- Hits        47967    47966       -1     
  Misses       3999     3999
Flag Coverage Δ
#multiple 90.71% <100%> (-0.01%) ⬇️
#single 43% <33.33%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/indexes/accessors.py 91% <100%> (-0.09%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1905485...8326b9a. Read the comment docs.

jreback
jreback requested changes Dec 25, 2018
View reviewed changes
pandas/tests/series/test_datetime_values.py
@@ -485,6 +485,13 @@ def test_dt_accessor_invalid(self, ser):
ser.dt
assert not hasattr(ser, 'dt')

def test_dt_accessor_updates_on_inplace(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you test this for the other accessors as well. I think you can assert that the result.index is not the s.index to test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can leave this test and add some in pandas/series/test_api.py

@jreback jreback added the Bug label Dec 25, 2018
@jreback jreback added this to the 0.24.0 milestone Dec 26, 2018
@jreback
Copy link
Contributor

jreback commented Dec 26, 2018

lgtm. ping on green.

@JustinZhengBC
Copy link
Contributor Author

@jreback green

@jreback jreback merged commit ef1bd69 into pandas-dev:master Dec 26, 2018
@jreback
Copy link
Contributor

jreback commented Dec 26, 2018

thanks!

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
@JustinZhengBC @Pingviinituutti
BUG-24408 Series.dt does not maintain own copy of index (pandas-dev#2…
11aa4c1 
…4426)
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
@JustinZhengBC @Pingviinituutti
BUG-24408 Series.dt does not maintain own copy of index (pandas-dev#2…
fef803a 
…4426)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
0.24.0
Development

Successfully merging this pull request may close these issues.

BUG: Series.dt accessor throws exception after Series.drop when inplace=True
2 participants
@JustinZhengBC @jreback